Add support for floatmax(value) and friends #746
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #746 +/- ##
==========================================
- Coverage 89.57% 86.99% -2.59%
==========================================
Files 11 10 -1
Lines 969 961 -8
==========================================
- Hits 868 836 -32
- Misses 101 125 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I just checked, and the docstring of typemin, typemax, floatmin, and floatmax only documents that these function can be called with types. A non-Type argument does not seem to be documented, so maybe it's not intended to be used with values? For comparison, the docstring for eps documents both the version with values and types.
The Julia docs even discourage the definition of functions for both value and type arguments: https://docs.julialang.org/en/v1/manual/style-guide/#Avoid-confusion-about-whether-something-is-an-instance-or-a-type
Given the documentation of these functions, I'd think the better solution would be to change https://github.com/JuliaMath/SpecialFunctions.jl/blob/c400278f3b358aea817e03bfdf9e23c3c08557e5/src/expint.jl#L169 to scale = sqrt(floatmax(typeof(real(A)))).
I'm fine with both approaches. @ararslan suggested in JuliaMath/SpecialFunctions.jl#475 to implement it in ForwardDiff, since other might be using the undocumented behavior from Base. |
|
My feeling is that it would be preferable if it would be documented that this function is expected to work on values before adding these definitions in ForwardDiff. |
|
Closed in favour of JuliaMath/SpecialFunctions.jl#494. |
2 participants
Fix JuliaMath/SpecialFunctions.jl#475
The SpecialFunction
expintis usingfloatmaxon a value and thus the following currently fails:I believe it makes sense to support both
floatmax(::T)andfloatmax(::Type{T})here since Base implements both.